Feature/stateless 2.0 - #15

Review (do not merge) (changed PR to DRAFT)

HEAD 283d91a. This is a parallel MCP 2.0 stack bolted onto the existing SDK, not a surgical extension of TaskManager + HTTP transport + auth. CI passing on sidecar tests is not enough.

Load-bearing assumptions (please confirm or drop)

These were never stated, and most of the +5.8k lines depend on them:

1. MCP 2026-07-28 requires a new JSON-RPC parser, ingress pipeline, task type system, and ticket/blueprint registry — rather than extending mcp.server.lowlevel, TaskManager, and transports/http.py.
2. Flipping ServerConfig.stateless default False → True is a safe default, not a breaking session-semantics change.
3. Scanning controllers only for @tool is equivalent to scanning the whole container (providers with @tool may disappear from discovery).

If (1) is wrong, delete the sidecar. If (2)/(3) are unintentional, revert those drive-bys.

Blockers

1. Two runtimes. protocol/jsonrpc.py + StatelessIngressPipeline + ASGI middleware re-parse JSON-RPC, then handle_post often returns None so mcp.server.lowlevel can handle the same request. wrap_stateless_transport is a sidecar, not the product path.

Code judo: register ping / server/discover / reject deprecated tasks/* on the existing low-level server. Delete the second parser, DispatchStage, RuntimeLayer, and the replay/synthetic-disconnect middleware.

2. Ticket registries in the package. runtime/conformance.py and runtime/epic_acceptance.py encode Plane tickets as importable tuples. Tests assert import_module works and len(ACCEPTANCE_CRITERIA) >= 8. doc11 looks for markdown under ../traker/stateless-doc-for-python. That is not “task-augmented tools/call returns TaskData.” Several “conformance” tests hit the pipeline directly and never TestClient + McpApplication.

Success criteria: one behavioral test per epic through get_combined_app / _call_tool. If a module exists only so verify_package_layout passes, delete it.

3. core/app.py growth + spaghetti. File was already >1k lines. MRTR, cache hints, traces, task access, TTL ms, wrap, and deprecating tasks/list+tasks/result all landed here. Discovery changed to controllers only. That is feature logic leaking into the shared bootstrap path.

4. Duplicate / leaky contracts. TaskStatus enum vs Literal; TaskData vs TaskWireData/TaskEntry; dual TTL (ttl_seconds / ttl_ms); two _meta parsers. check_task_access allows all when context is None, and only mismatches when both sides set an id. JWT errors swallowed (except Exception: pass). created_at: Any.

5. Breaking / incidental behavior. stateless default True; OPTIONS 204 on all paths; body replay + synthetic disconnect. Thin wrappers (normalize_headers identity, 27-line sse.py, RuntimeLayer unused in any real caller).

Mergeable shape (split PRs, each with a real HTTP/tool test)

1. Task store + input_required + TTL-on-terminal inside existing TaskManager.
2. Stateless / no Mcp-Session-Id inside current HTTP transport; keep stateless=False as default unless this is an intentional major-version break.
3. CIMD in auth/ only.
4. Cache / trace / MRTR as small helpers used by _call_tool, not a protocol/ package.
5. No runtime/conformance.py, no epic_acceptance.py.

Until then this is a spec transcript, not an SDK change.

Reviewed HEAD 283d91a (feature/stateless_2.0 → develop). Agree with the existing architecture comment: this is still a parallel MCP 2.0 sidecar, and it should not merge as-is. Below are concrete bugs verified on the branch, in addition to that write-up.

Blockers

1. Task isolation is deny-by-accident, not deny-by-default
nitrostack/tasks/authorization.py

if context is None:
    return
if entry.tenant_id and context.tenant_id and entry.tenant_id != context.tenant_id:
    raise TaskNotFoundError(task_id)

Two production holes:

• extract_task_access_context() returns None when the client sends no identity _meta (the normal path). check_task_access then allows every task.
• Checks only fire when both sides have an id. A task stored with tenant_id=acme, owner_id=alice is readable by _meta.userId=alice with no tenantId.

tests/test_mcp20_doc07_task_authorization.py even locks this in (test_none_context_allows_internal_access). Doc 07 “anti-enumeration” is not met for missing/partial context.

Fix: If the task was created with owner/tenant/session, require a matching verified context on every get/cancel/list. Missing dimension → TaskNotFoundError. Keep None only for a clearly internal, non-HTTP code path — not tasks/get.

2. Task identity is client-spoofable; JWT failures are swallowed
nitrostack/tasks/authorization.py

user_id = meta.get("userId") or meta.get("user_id")
# ...
payload = ...verify_token(token)
user_id = user_id or payload.get("sub")
except Exception:
    pass

_meta.userId / tenantId are trusted before JWT, and verify errors are ignored. Anyone who can guess/learn a taskId can impersonate the owner. Identity must come from the HTTP auth middleware / verified JWT, never unsigned request meta. If Authorization is present and verify fails, deny.

3. CIMD DNS check then reconnect-by-hostname is classic rebinding SSRF
nitrostack/auth/cimd.py

assert_safe_fetch_target() getaddrinfos and blocks RFC6890 ranges, then _fetch_cimd_bytes() opens the hostname again with urllib. A second lookup can land on 169.254.169.254 / RFC1918. Redirects are blocked; rebinding is not. Doc 08 tests patch _fetch_cimd_bytes, so they never hit this path.

Also: resolve_cimd / validate_authorization_iss are not called from oauth_module / token flows. The OAuth+CIMD epic is an unused helper plus a comment that DCR is “deprecated.”

Fix: Resolve once, connect to the allowed IP (pin Host), re-check the peer address. Wire CIMD into registration or stop claiming Doc 08 done.

4. Synthetic http.disconnect after body replay will abort Streamable HTTP
nitrostack/transports/middleware.py and _replay_receive

Because stateless now defaults to True, almost every POST /mcp is buffered, parsed by the sidecar, then replayed. The replayed receive channel returns http.disconnect on the second call. Streamable HTTP (default json_response=False) listens for disconnect to tear down SSE. Forwarded tools/call / initialize can look like the client hung up immediately.

This is not covered by the pipeline unit tests (handle_post only). Need a real TestClient (or ASGI) test that a forwarded tools/call still completes.

5. OPTIONS 204 on every path
StatelessTransportMiddleware.__call__

Any OPTIONS — /mcp/health, widgets, /sse — is short-circuited to 204 and never reaches Starlette. Scope this to MCP paths, or compose with the existing CORS in transports/http.py instead of wrapping the whole app.

High (breaking / incomplete wiring)

6. ServerConfig.stateless default False → True
nitrostack/core/app.py L79

Silent session-semantics break for every app that did not set MCP_STATELESS / stateless=. Keep False unless this is an intentional major version.

7. Discovery now scans controllers only
nitrostack/core/app.py L393–406

develop scanned container._instances (providers + controllers). Providers with @tool / @resource / @prompt / health / events disappear with no warning. Restore provider scan (or container-wide) and add a regression test.

8. Ingress never installs task/registry handlers
wrap_stateless_transport

StatelessIngressPipeline is constructed with no task_handler. is_task_wire_interception is dead in production; tasks/* and task-augmented tools/call always fall through. Either wire real handlers or delete the interception stage so there is one task path (TaskManager + low-level server).

9. TaskContext.update_progress / cancel fire-and-forget
nitrostack/core/context.py

asyncio.create_task(manager.update_progress(...))
except Exception:
    pass

Store updates are not awaited; failures (including “no running loop”) are swallowed. Progress and cancel will flake. Await on the tool’s event loop, or make these async.

10. @tool task_support default forbidden → optional
nitrostack/core/decorators.py

Every existing tool becomes task-augmentable. Restore forbidden; opt in per tool.

Medium

11. CORS reflects any Origin
nitrostack/transports/cors.py with Authorization in allow-headers. Allowlist origins; do not echo arbitrary Origin in production.

12. Wrong JSON-RPC error code for invalid requests
parse_jsonrpc_request uses -32700 for non-object body, bad jsonrpc, bad method/params. Those are -32600 Invalid Request. Reserve parse error for decode failures.

13. Duplicate task contracts
core/task.py (TaskStatus Enum, TaskData) vs tasks/types.py (TaskStatus Literal, TaskWireData/TaskEntry) vs MCP SDK CreateTaskResult. Dual TTL (ttl_seconds / ttl_ms). Pick one model at the handler boundary.

14. Schema depth walker treats single-schema keywords as maps
bound_schema_depth iterates items / not / additionalProperties dict keys as nested schemas. items: {type: string} is one schema, not a properties-style map.

15. Ticket registries in the installable package
runtime/conformance.py and runtime/epic_acceptance.py are Plane/Doc checklists (import_module + len(ACCEPTANCE_CRITERIA) >= 8). They do not prove tools/call returns TaskData over HTTP. Move to tests/ / docs; don’t export from nitrostack.

Tests

Almost all new tests are helper/registry unit tests. I only see a real TestClient POST /mcp in test_mcp20_doc10_blueprint.py (echo tool). Missing: middleware + forwarded tools/call, task isolation over HTTP headers/JWT, CIMD without mocks, provider-defined @tool still discovered, stateless=True vs sessionful clients.

Until isolation, CIMD pinning, replay/disconnect, and the stateless/discovery/task_support defaults are fixed, this should stay draft. Prefer splitting: TaskStore inside existing TaskManager, stateless inside current HTTP transport (stateless=False default), CIMD in auth/ and actually wired, no sidecar parser, no epic registry modules.